Skip to content

fs: accept all valid utf8 values in fast paths#62304

Open
wdskuki wants to merge 2 commits intonodejs:mainfrom
wdskuki:fix-utf8-fast-paths
Open

fs: accept all valid utf8 values in fast paths#62304
wdskuki wants to merge 2 commits intonodejs:mainfrom
wdskuki:fix-utf8-fast-paths

Conversation

@wdskuki
Copy link

@wdskuki wdskuki commented Mar 18, 2026

This PR fixes issue #49888 by accepting all valid UTF8 encoding variants (utf8, utf-8, UTF8, UTF-8) in fs.readFileSync and fs.writeFileSync fast paths.

Changes

  • Modified lib/fs.js to accept UTF8 and UTF-8 (uppercase) in addition to utf8 and utf-8
  • Added test case to verify the fix

Testing

  • Added test/parallel/test-fs-utf8-fast-path-casing.js

Fixes: #49888

Accept UTF8 and UTF-8 (uppercase) in fs.readFileSync and
fs.writeFileSync fast paths, in addition to utf8 and utf-8.

Fixes: nodejs#49888
@nodejs-github-bot nodejs-github-bot added fs Issues and PRs related to the fs subsystem / file system. needs-ci PRs that need a full CI run. labels Mar 18, 2026
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Copy link
Member

@jasnell jasnell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM modulo the linting issues in the test

@codecov
Copy link

codecov bot commented Mar 19, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.68%. Comparing base (9fc6b64) to head (7f98286).
⚠️ Report is 10 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #62304   +/-   ##
=======================================
  Coverage   89.68%   89.68%           
=======================================
  Files         676      676           
  Lines      206575   206577    +2     
  Branches    39549    39553    +4     
=======================================
+ Hits       185262   185274   +12     
+ Misses      13446    13445    -1     
+ Partials     7867     7858    -9     
Files with missing lines Coverage Δ
lib/fs.js 98.19% <100.00%> (+<0.01%) ⬆️

... and 34 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

The fs.readFileSync and fs.writeFileSync fast paths for UTF-8 encoding
only accepted exact matches: 'utf8', 'utf-8', 'UTF8', 'UTF-8'. However,
Node.js considers all case-insensitive variations as valid UTF-8 (e.g.,
'Utf8', 'Utf-8', 'uTf8', etc.).

This commit adds an isUtf8Encoding() helper function that accepts all
valid UTF-8 encoding names, matching the behavior of normalizeEncoding()
in lib/internal/util.js.

Fixes nodejs#49888
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fs Issues and PRs related to the fs subsystem / file system. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fs: utf8 fast paths don't accept all valid utf8 values

4 participants